Skip to content

refactor(tests): split inline test mods into sibling tests.rs files + 2 PR #20 review fixes#21

Merged
uqio merged 2 commits intomainfrom
chore/split-test-mods
Apr 26, 2026
Merged

refactor(tests): split inline test mods into sibling tests.rs files + 2 PR #20 review fixes#21
uqio merged 2 commits intomainfrom
chore/split-test-mods

Conversation

@uqio
Copy link
Copy Markdown
Collaborator

@uqio uqio commented Apr 26, 2026

Mechanical cleanup PR. Moves the inline #[cfg(all(test, feature = "std"))] mod tests { ... } blocks out of seven large source files (one per file) into sibling tests.rs files, and folds in two unmerged Copilot review fixes from PR #20. Zero behavior change — pure relocation + two doc/codegen tweaks.

Driven by a long-running file-size concern: src/sinker/mixed.rs had grown to 13,758 lines (of which ~5,900 were inline tests), and each per-arch SIMD backend was in the 5,000–5,700-line range. IDE navigation, code review, and git blame all suffered. Splitting tests into siblings shrinks the production-facing surface by 36% on average across the seven files without changing what runs in CI.

Mechanics

Rust 2024 edition supports the foo.rs + foo/ submodule layout natively. For each source file <dir>/<stem>.rs:

  1. The inline #[cfg(all(test, feature = "std"))] mod tests { ... } block is replaced with the one-line declaration #[cfg(all(test, feature = "std"))] mod tests;.
  2. The block body moves verbatim to <dir>/<stem>/tests.rs. use super::*; continues to resolve the same way (the new file is still inside the parent module's namespace).
  3. #[cfg_attr(miri, ignore = ...)] and other per-test attributes carry through unchanged.

The wasm backend keeps its target_feature = "simd128" cfg gate (#[cfg(all(test, feature = "std", target_feature = "simd128"))] mod tests;).

What's in this PR

Splits (7 files → 7 sibling test files)

Source file Before After (production) Sibling test file (lines / #[test] markers)
src/sinker/mixed.rs 13,758 7,860 src/sinker/mixed/tests.rs (5,897 / 188)
src/row/scalar.rs 2,565 2,075 src/row/scalar/tests.rs (489 / 33)
src/row/arch/neon.rs 5,611 3,568 src/row/arch/neon/tests.rs (2,042 / 67)
src/row/arch/x86_sse41.rs 5,073 3,254 src/row/arch/x86_sse41/tests.rs (1,818 / 62)
src/row/arch/x86_avx2.rs 5,551 3,768 src/row/arch/x86_avx2/tests.rs (1,782 / 62)
src/row/arch/x86_avx512.rs 5,736 3,937 src/row/arch/x86_avx512/tests.rs (1,798 / 62)
src/row/arch/wasm_simd128.rs 5,130 3,560 src/row/arch/wasm_simd128/tests.rs (1,569 / 58)

Total: 15,402 lines of test code relocated; production-facing surface dropped by ~36% on average across the seven files. Test count unchanged (475 lib tests still discovered + run on aarch64).

Copilot review fixes from PR #20 (folded in)

Two of the four findings on Copilot review #4176623587 were applied to PR #20 locally but never made it into the merge commit. Folded in here since they're trivial and adjacent:

What's deferred (still in the cleanup memory)

The other two PR #20 review findings stay deferred to future cleanup PRs because addressing them properly requires a sweep beyond this PR's scope:

  • Finding feat(NV12): NV12(semi-planar 4:2:0) + fallible PixelSink contract #2 — visibility tightening on pub(crate) fn nv12_or_nv21_to_rgb_or_rgba_row_impl<...> and pub(crate) fn nv24_or_nv42_to_rgb_or_rgba_row_impl<...> in src/row/scalar.rs, plus the corresponding SIMD pub(crate) unsafe fn *_to_rgb_or_rgba_row_impl functions across all 5 backends. Each is only called from its own file's wrappers — they should all be fn (module-private). Single uniform sweep, no behavior change. Defer because mixing visibility tightening into this PR would obscure the test-mod relocation diff.
  • Finding feat(yuv420p10): 10-bit YUV 4:2:0 planar → u8 + native u16 RGB #4 — extract a small rgba_plane_slice helper to remove the duplicated one_plane_end.checked_mul(4)? + one_plane_start * 4 + slice pattern across 8 MixedSinker<F>::process impls (Yuv420p / Yuv422p / Yuv444p / Nv12 / Nv16 / Nv21 / Nv24 / Nv42), in both the standalone-RGBA branch and the Strategy A expand branch. Parallel rgb_plane_slice (3-channel + scratch fallback) helper should ship at the same time. Worth doing before tranches 5–7 (high-bit-depth RGBA) start landing — those will multiply the duplication.

The cleanup memory tracks both of these for follow-up work.

What's NOT in scope here (intentionally)

  • No production-code changes beyond the two tiny Copilot fixes above.
  • No test logic changes — pure mechanical relocation. No renames, no skips, no coverage shifts.
  • No per-arch test helper consolidation (high_bit_plane_*, interleave_uv_*, etc., duplicated across the 5 backend test files). Already noted in docs/color-conversion-functions.md § Cleanup follow-ups; would belong in a sibling refactor PR.
  • No reordering or grouping inside the moved test files — they're verbatim from the source files. Reorganizing is a separate concern.

Verification

  • cargo test --lib475 passed; 0 failed (same as pre-refactor baseline)
  • cargo test --doc — 1 passed (the compile_fail doctest on MixedSinker::<Yuv420p>::with_rgba still rejects MixedSinker::<Yuv440p>::with_rgba)
  • cargo check --lib --target wasm32-unknown-unknown — clean
  • cargo check --lib --target x86_64-unknown-linux-gnu — clean
  • cargo clippy --lib --tests — only one pre-existing warning in src/raw/bayer.rs (unrelated unnecessary_cast)

Test plan

  • CI green on test, test-sde-avx512, cross, coverage, clippy, build, miri-* jobs.
  • Per-tier coverage matrix exercises every backend's tests via colconv_disable_* rustflags (the relocation should be transparent to the matrix).
  • Spot-check the new file layout: src/sinker/mixed/tests.rs, src/row/arch/neon/tests.rs, etc. resolve under both cargo test --lib and IDE jump-to-test.

🤖 Generated with Claude Code

@al8n al8n changed the title update refactor(tests): split inline test mods into sibling tests.rs files + 2 PR #20 review fixes Apr 26, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 26, 2026

Codecov Report

❌ Patch coverage is 83.30206% with 356 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/row/arch/neon/tests.rs 80.14% 106 Missing ⚠️
src/row/arch/x86_avx512/tests.rs 83.58% 87 Missing ⚠️
src/row/arch/x86_sse41/tests.rs 83.98% 86 Missing ⚠️
src/row/arch/x86_avx2/tests.rs 85.30% 77 Missing ⚠️

📢 Thoughts on this report? Let us know!

@uqio uqio merged commit 9596d14 into main Apr 26, 2026
3 of 66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant